Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v10.15.0 proposal #25176

Merged
merged 10 commits into from
Dec 26, 2018
Merged

v10.15.0 proposal #25176

merged 10 commits into from
Dec 26, 2018

Conversation

MylesBorins
Copy link
Contributor

@MylesBorins MylesBorins commented Dec 21, 2018

2018-12-26, Version 10.15.0 'Dubnium' (LTS), @MylesBorins

The 10.14.0 security release introduced some unexpected breakages on the 10.x release line.
This is a special release to fix a regression in the HTTP binary upgrade response body and add
a missing CLI flag to adjust the max header size of the http parser.

Notable Changes

  • cli:
    • add --max-http-header-size flag (cjihrig) #24811
  • http:
    • add maxHeaderSize property (cjihrig) #24860

Commits

  • [9b2ffc81c0] - (SEMVER-MINOR) cli: add --max-http-header-size flag (cjihrig) #24811
  • [6183c7107d] - (SEMVER-MINOR) deps: cherry-pick http_parser_set_max_header_size (cjihrig) #24811
  • [e669733595] - doc: describe current HTTP header size limit (Sam Roberts) #24700
  • [b6d3afb257] - (SEMVER-MINOR) http: add maxHeaderSize property (cjihrig) #24860
  • [1aea1e3634] - http: fix regression of binary upgrade response body (Matteo Collina) #25039
  • [a57aed144a] - (SEMVER-MINOR) src: add kUInteger parsing (Matteo Collina) #24811
  • [527407c49f] - src: cache the result of GetOptions() in JS land (Joyee Cheung) #24091
  • [728bc631e5] - test: fix expectation in test-bootstrap-modules (Ali Ijaz Sheikh) #25112
  • [3e14212f0e] - test: remove magic numbers in test-gc-http-client-onerror (Rich Trott) #24943

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. v10.x labels Dec 21, 2018
MylesBorins added a commit that referenced this pull request Dec 21, 2018
The 10.14.0 security release introduced some unexpected breakages on
the 10.x release line. This is a special release to fix a regression
in the HTTP binary upgrade response body and add a missing CLI flag
to adjust the max header size of the http parser.

Notable Changes:

* cli:
  - add --max-http-header-size flag (cjihrig)
    #24811

PR-URL: #25176
@MylesBorins
Copy link
Contributor Author

@MylesBorins
Copy link
Contributor Author

MylesBorins commented Dec 22, 2018

@MylesBorins

This comment has been minimized.

MylesBorins added a commit that referenced this pull request Dec 22, 2018
The 10.14.0 security release introduced some unexpected breakages on
the 10.x release line. This is a special release to fix a regression
in the HTTP binary upgrade response body and add a missing CLI flag
to adjust the max header size of the http parser.

Notable Changes:

* cli:
  - add --max-http-header-size flag (cjihrig)
    #24811

PR-URL: #25176
MylesBorins added a commit that referenced this pull request Dec 22, 2018
The 10.14.0 security release introduced some unexpected breakages on
the 10.x release line. This is a special release to fix a regression
in the HTTP binary upgrade response body and add a missing CLI flag
to adjust the max header size of the http parser.

Notable Changes:

* cli:
  - add --max-http-header-size flag (cjihrig)
    #24811

PR-URL: #25176
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Trott and others added 7 commits December 25, 2018 02:17
Remove magic numbers (500, 10, 100) from the test. Instead, detect when
GC has started and stop sending requests at that point.

On my laptop, this results in 16 or 20 requests per run instead of 500.

Fixes: #23089

PR-URL: #24943
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Instead of calling into C++ each time we need to check the value
of a command line option, cache the option map in a new
`internal/options` module for faster access to the values in JS land.

PR-URL: #24091
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Refael Ackermann <refack@gmail.com>
PR-URL: #25112
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
PR-URL: #25039
Fixes: #24958
Reviewed-By: Myles Borins <myles.borins@gmail.com>
This commit adds http_parser_set_max_header_size() to the
http-parser for overriding the compile time maximum HTTP
header size.

Backport-PR-URL: #25168
PR-URL: #24811
Fixes: #24692
Refs: nodejs/http-parser#453
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
This commit adds support for uint64_t option parsing.

Backport-PR-URL: #25168
PR-URL: #24811
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Allow the maximum size of HTTP headers to be overridden from
the command line.

Backport-PR-URL: #25168
co-authored-by: Matteo Collina <hello@matteocollina.com>
PR-URL: #24811
Fixes: #24692
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins added a commit that referenced this pull request Dec 25, 2018
The 10.14.0 security release introduced some unexpected breakages on
the 10.x release line. This is a special release to fix a regression
in the HTTP binary upgrade response body and add a missing CLI flag
to adjust the max header size of the http parser.

Notable Changes:

* cli:
  - add --max-http-header-size flag (cjihrig)
    #24811

PR-URL: #25176
@MylesBorins
Copy link
Contributor Author

CI: https://ci.nodejs.org/job/node-test-pull-request/

Added fix to flaky test. CITGM looks good

sam-github and others added 2 commits December 25, 2018 09:48
Document that the limit was changed from 80KB to 8KB in 1860352.

Fixes: #24693

PR-URL: #24700
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
This commit exposes the value of --max-http-header-size
as a property of the http module.

PR-URL: #24860
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The 10.14.0 security release introduced some unexpected breakages on
the 10.x release line. This is a special release to fix a regression
in the HTTP binary upgrade response body and add a missing CLI flag
to adjust the max header size of the http parser.

Notable Changes:

* cli:
  - add --max-http-header-size flag (cjihrig)
    #24811
* http:
  - add maxHeaderSize property (cjihrig)
    #24860

PR-URL: #25176
@MylesBorins
Copy link
Contributor Author

MylesBorins commented Dec 25, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/19806/ ✅ (only failure is known flake on AIX, retrying)
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1703/ ✅ (manually tested failures and updated skip / flaky references in citgm lookup)
rc.2: https://nodejs.org/download/rc/v10.15.0-rc.2/

@MylesBorins MylesBorins merged commit 1ae0511 into v10.x Dec 26, 2018
MylesBorins added a commit that referenced this pull request Dec 26, 2018
MylesBorins added a commit that referenced this pull request Dec 26, 2018
The 10.14.0 security release introduced some unexpected breakages on
the 10.x release line. This is a special release to fix a regression
in the HTTP binary upgrade response body and add a missing CLI flag
to adjust the max header size of the http parser.

Notable Changes:

* cli:
  - add --max-http-header-size flag (cjihrig)
    #24811
* http:
  - add maxHeaderSize property (cjihrig)
    #24860

PR-URL: #25176
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
The 10.14.0 security release introduced some unexpected breakages on
the 10.x release line. This is a special release to fix a regression
in the HTTP binary upgrade response body and add a missing CLI flag
to adjust the max header size of the http parser.

Notable Changes:

* cli:
  - add --max-http-header-size flag (cjihrig)
    nodejs#24811
* http:
  - add maxHeaderSize property (cjihrig)
    nodejs#24860

PR-URL: nodejs#25176
@codebytere codebytere mentioned this pull request Jan 15, 2019
@targos targos deleted the v10.15.0-proposal branch June 4, 2019 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants